Ensure Serializer runtime exceptions are rethrown as IOException#6969
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6969 +/- ##
============================================
+ Coverage 90.09% 90.11% +0.02%
Complexity 6600 6600
============================================
Files 730 730
Lines 19843 19852 +9
Branches 1955 1955
============================================
+ Hits 17877 17890 +13
+ Misses 1371 1367 -4
Partials 595 595 ☔ View full report in Codecov by Sentry. |
laurit
left a comment
There was a problem hiding this comment.
Looks good to me. I don't see too many alternatives. I guess instead of UncheckedIOException could use a sneaky throw hack and throw the IOException without declaring it, but the current solution is probably easier to understand. Another alternative would be to reimplement the forEach and introduce a ThrowingConsumer, but that isn't desirable because we'd need to use iterator in our forEach variant while IdentityHashMap etc. implement it without extra allocations.
One other option I thought of was to add a
How does this work?
I considered this as well and reached the same conclusion. In the past, benchmarks have shows extra memory allocation using an iterator vs. |
I believe the most common way is to use generics together with unchecked cast like in https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/cf0f530dc177951f457ffc2bc21c61382d4a10b7/instrumentation/reactor/reactor-3.1/library/src/main/java/io/opentelemetry/instrumentation/reactor/v3_1/ContextPropagationOperator.java#L250 which exploits generics and an unchecked cast to trick compiler to think that you are actually throwing a runtime exception. Here someone tracked down the part of the jls that makes this possible. I first saw this in https://www.google.com/search?q=java+puzzlers, for some reason I think that @breedx-splk has the book, idk why I remember it, could be wrong. Another way this could be done is using |
Wow I didn't know that was possible. Fascinating! I think the approach in this PR is most straight forward to understand and without seemingly any technical downside. So let's go with this, but thanks for sharing! I trust compile time errors with unhandled checked exceptions a little bit less now that I know that the absence of |
Resolves #6946, #6916.
@laurit interested in your thoughts on this as well, since its about the stateless marshalers.